Skip to content

fix: don't rescope imported names #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jquense
Copy link
Contributor

@jquense jquense commented Feb 15, 2019

I think i would consider this a fix. But maybe we should communicate more explicitly with the values plugin via result

the idea here is if you have an imported value, that values identifier shouldn't be changed in the places it was replaced other downstream the right values won't work

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid ; too.

Also please create issue for coordination in future. Now we have 3 PRs and can potential break other code. We should discussion and merge step by step. No need rush.

@jquense
Copy link
Contributor Author

jquense commented Feb 15, 2019

Sorry @evilebottnawi it's not clear where to open an issue for all of these packages broadly. I did try to bring this up for discussion here: css-modules/postcss-modules-values#2 (comment) sorry if i wasn't clear there 👍

@jquense
Copy link
Contributor Author

jquense commented Feb 15, 2019

it may be worth consolidating a lot of these small plugins into a single postcss-css-modules monorepo

@alexander-akait
Copy link
Member

@jquense i think it is impossible right now due historical reason, let's solve some architecture problem in plugin first and then we can do this

@alexander-akait
Copy link
Member

/cc @jquense can you rebase?

@jquense
Copy link
Contributor Author

jquense commented Feb 20, 2019

i can, tho I think that i may close this one for a different, better approach. I rather than skipping values, it would be better to not scope them as :local in the first place.

@alexander-akait
Copy link
Member

@jquense 👍 Feel free to do it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants